Require Selenium for browser-owned Components regressions - #69064
Require Selenium for browser-owned Components regressions#69064PureWeen wants to merge 7 commits into
Conversation
Document the permanent regression boundary, align the interactive validation skill handoff, and add focused Vally coverage for browser and lower-boundary test selection. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e2b46a36-410e-48dc-8ce7-cf07dab0bc96
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e2b46a36-410e-48dc-8ce7-cf07dab0bc96
|
/evaluate 1d6c41b |
|
Hey @dotnet/aspnet-build, looks like this PR is something you want to take a look at. |
There was a problem hiding this comment.
Copilot review overview
🟢 Approval recommended
The changes are scoped to guidance/skill/eval content, and the new instructions reference existing repo paths and mechanisms consistently without introducing correctness or integration risks.
Review tier: Lite
Findings: None
What changed in this PR
This PR updates contributor guidance and the validate-blazor-feature skill workflow to require real-browser (Selenium) coverage for browser-owned Components regressions, and to formalize a deterministic Playwright → diagnostic JS probe → Selenium handoff process (including a five-field “boundary” record) before selecting permanent coverage.
Changes:
- Expanded
src/Components/AGENTS.mdimplementation workflow with deterministic reproduction guidance and a “Permanent regression test boundary” policy (including the Selenium/Jest split and synchronization guidance). - Updated
.github/skills/validate-blazor-feature/SKILL.mdto incorporate the deterministic probe + boundary-recording workflow and clarify the permanent-test handoff expectations. - Extended
eng/skill-evals/validate-blazor-feature/eval.vally.yamlwith new stimuli/rubrics covering the new boundary and determinism policies, and adjusted scoring weights.
| File | Description |
|---|---|
| src/Components/AGENTS.md | Adds explicit determinism workflow and a normative permanent regression test boundary (Selenium vs Jest/.test.ts guidance). |
| eng/skill-evals/validate-blazor-feature/eval.vally.yaml | Updates eval weighting and adds new stimuli/rubrics to enforce the revised policy and decision-making. |
| .github/skills/validate-blazor-feature/SKILL.md | Aligns skill instructions with the new deterministic reproduction + handoff + boundary-record workflow. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
|
✅ Smoke skill evaluation passed for |
Summary
This implements Ilona's maintainer feedback on #68748 and adds a focused deterministic browser-to-Selenium workflow:
src/Components/test/E2ETest.TaskCompletionSourceonly when deterministic managed or intermediate ordering must be controlled; it does not stabilize races with an arbitrary fixed sleep.Browser.True/Browser.Equal(WebDriverWait) is sufficient when the final observable can be awaited directly; an explicit gate is not ceremonial. Existing bounded polling helpers may delay between checks, so this is not a syntactic ban on everyThread.Sleep.output-not-matchesgraders and moves semantic denials/permissions to prompt-judge rubrics. Remaining static graders check only broad surface signals; five-field decisions are judged for substantive case-specific values rather than placeholder labels.This PR changes contributor guidance, the existing interactive validation skill, and its eval lane only; it does not change production or test implementation code.
Validation
.github/skills/validate-blazor-featurepasses strict skill lint.eng/skill-evals/run.ps1 Validate -Eval eng/skill-evals/validate-blazor-feature/eval.vally.yamlpasses repository layout checks, Vally 0.14 strict lint/schema validation, and the model-free experiment dry-run.eng/skill-evals/run.ps1 Lint -Eval eng/skill-evals/validate-blazor-feature/eval.vally.yamlpasses the pinned Vally 0.14 strict lint.git diff --checkpasses.Skill-creator comparison
The pre-change skill was snapshotted outside the repository and compared with the revised workflow across four paired Claude Sonnet 5 cases: deterministic browser handoff, nondeterministic producer investigation, arbitrary sleep rejection, and manager-contract versus browser-reachability selection.
Bounded model-bearing Vally evidence and limitations
A focused baseline-versus-skilled run used Claude Opus 4.6 for three trials each across four workflow cases (12 trajectories per variant):
TaskCompletionSource, which is the intended policy. The model did not invoke the skill for those three straightforward sleep prompts, but the behavior was correct; the other nine skilled trials invokedvalidate-blazor-feature.